fix(detector): fail closed on GPU health check errors - #26
Merged
Conversation
- replace the blanket suppress in _get_memory_status with an explicit catch: tolerate NVML_ERROR_NOT_SUPPORTED, any other NVMLError reports the device unhealthy - probe NVML_FI_DEV_GET_GPU_RECOVERY_ACTION and NVML_FI_DEV_RESET_STATUS so a GSP failure (Xid 119/154) that leaves the ECC counters readable still reports unhealthy; an unreadable field falls back to the ECC verdict - add regression tests over the fake pynvml, including the fail-closed cases that previously reported healthy Task 1-3 of nvidia-gsp-failure-reports-healthy. Signed-off-by: thxCode <thxcode0824@gmail.com>
- GPUSTACK_RUNTIME_DETECT_NO_HEALTH_CHECK: with the check on, NVIDIA health also covers the driver recovery state (GPU reset required), an errored query reports unhealthy, and a query against a wedged device can block until the driver's RPC timeout (up to 45s with GSP firmware) - record the same in the detector-alignment spec's F1 all-vendors row Task 4 of nvidia-gsp-failure-reports-healthy. Signed-off-by: thxCode <thxcode0824@gmail.com>
- extract _get_memory_status from the two inline health checks and replace the blanket suppress: tolerate NVML_ERROR_NOT_SUPPORTED, any other NVMLError from ixmlDeviceGetHealth reports the device unhealthy Task 5 of nvidia-gsp-failure-reports-healthy. Signed-off-by: thxCode <thxcode0824@gmail.com>
- replace the blanket suppress in _get_memory_status: tolerate HGML_ERROR_NOT_SUPPORTED, any other HGMLError from the ECC counter query reports the device unhealthy Task 6 of nvidia-gsp-failure-reports-healthy. Signed-off-by: thxCode <thxcode0824@gmail.com>
- extract _get_memory_status from the two inline health checks: an AMD SMI ECC error with a genuine status code reports the device unhealthy, AMDSMI_STATUS_NOT_SUPPORTED keeps the previous verdict, and the codeless stub error falls back to ROCm SMI as the memory read does - the ROCm SMI path follows the same policy, tolerating RSMI_STATUS_NOT_SUPPORTED and a missing ECC symbol Task 7 of nvidia-gsp-failure-reports-healthy. Signed-off-by: thxCode <thxcode0824@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors and improves the device memory health check logic across multiple GPU backends (AMD, Iluvatar, NVIDIA, and THead) to implement a fail-closed policy where genuine driver query errors mark the device as unhealthy, while unsupported queries are tolerated. On NVIDIA, the check is enhanced to also probe the driver's recovery state (such as awaiting reset after a GSP failure). Comprehensive unit tests have been added to verify these behaviors under various failure scenarios. There are no review comments, so I have no feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On an 8x H100 node, a card whose GSP firmware stopped answering RPCs (Xid 119 timeout → Xid 154 GPU Reset Required) showed
ERR!/N/Atelemetry innvidia-smi, yetgpustack-runtime detectstill reported it healthy. The health check is opt-in (GPUSTACK_RUNTIME_DETECT_NO_HEALTH_CHECKdefaults to true — unchanged), but even when enabled it was narrow and fail-open: the ECC query sat under a blanketcontextlib.suppress, so the very errors a wedged card produces were swallowed and the card reported healthy.The enabled health check now fails closed on every vendor carrying that pattern: a query the driver errors on reports the device unhealthy, while a query the hardware does not support keeps the previous verdict — no false positives on old drivers or ECC-less cards.
Changes
_get_memory_statuscatchesNVMLErrorexplicitly (NVML_ERROR_NOT_SUPPORTEDtolerated, anything else unhealthy) and additionally probesNVML_FI_DEV_GET_GPU_RECOVERY_ACTION/NVML_FI_DEV_RESET_STATUS, so a GSP failure that leaves the ECC counters readable still reports unhealthy; an unreadable field falls back to the ECC verdict_get_memory_statusfrom the two inline checks;ixmlDeviceGetHealtherrors fail closed the same wayHGML_ERROR_NOT_SUPPORTEDtolerated)_get_memory_status; the AMD SMI path decides pererr_code(the codeless stub error falls back to ROCm SMI, as the memory read does); the ROCm SMI path toleratesRSMI_STATUS_NOT_SUPPORTEDand a missing ECC symbolDefault behaviour (health check off) is unchanged: no extra driver calls per card per pass.
Verification
uv run pytest: 634 passed, 20 skipped;make lintcleanGPUSTACK_RUNTIME_DETECT_NO_HEALTH_CHECK=false gpustack-runtime detectshould report StatusERR